Skip to content

Conversation

Rajveer100
Copy link
Contributor

Resolves #123227

Previously, clippy was using cargo from PATH, but since PR, it now prioritises checking CARGO first.

@rustbot
Copy link
Collaborator

rustbot commented Apr 2, 2024

r? @clubby789

rustbot has assigned @clubby789.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Apr 2, 2024
@rust-log-analyzer

This comment has been minimized.

@onur-ozkan onur-ozkan added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 3, 2024
@Rajveer100 Rajveer100 force-pushed the branch-for-issue-123227 branch from 0150ec7 to cc4ed44 Compare April 3, 2024 10:56
@rust-log-analyzer

This comment has been minimized.

@Rajveer100 Rajveer100 force-pushed the branch-for-issue-123227 branch from cc4ed44 to 1e5adbd Compare April 3, 2024 11:09
@onur-ozkan
Copy link
Member

Thank you for working on this, but the current changes still don't seem correct. Here is the patch for the correct approach:

From c18be9da0560954cef2bde9e0fffa6ed215c9435 Mon Sep 17 00:00:00 2001
From: onur-ozkan <[email protected]>
Date: Wed, 3 Apr 2024 14:37:47 +0300
Subject: [PATCH] remove `PATH` and set `CARGO` for `clippy` cmd

Signed-off-by: onur-ozkan <[email protected]>
---
 src/bootstrap/src/core/builder.rs | 13 ++-----------
 1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/src/bootstrap/src/core/builder.rs b/src/bootstrap/src/core/builder.rs
index c051b818328..0cce5130f64 100644
--- a/src/bootstrap/src/core/builder.rs
+++ b/src/bootstrap/src/core/builder.rs
@@ -1194,20 +1194,11 @@ pub fn rustdoc(&self, compiler: Compiler) -> PathBuf {
     }
 
     pub fn cargo_clippy_cmd(&self, run_compiler: Compiler) -> Command {
-        let initial_sysroot_bin = self.initial_rustc.parent().unwrap();
-        // Set PATH to include the sysroot bin dir so clippy can find cargo.
-        // FIXME: once rust-clippy#11944 lands on beta, set `CARGO` directly instead.
-        let path = t!(env::join_paths(
-            // The sysroot comes first in PATH to avoid using rustup's cargo.
-            std::iter::once(PathBuf::from(initial_sysroot_bin))
-                .chain(env::split_paths(&t!(env::var("PATH"))))
-        ));
-
         if run_compiler.stage == 0 {
             // `ensure(Clippy { stage: 0 })` *builds* clippy with stage0, it doesn't use the beta clippy.
             let cargo_clippy = self.build.config.download_clippy();
             let mut cmd = Command::new(cargo_clippy);
-            cmd.env("PATH", &path);
+            cmd.env("CARGO", &self.initial_cargo);
             return cmd;
         }
 
@@ -1227,7 +1218,7 @@ pub fn cargo_clippy_cmd(&self, run_compiler: Compiler) -> Command {
 
         let mut cmd = Command::new(cargo_clippy);
         cmd.env(helpers::dylib_path_var(), env::join_paths(&dylib_path).unwrap());
-        cmd.env("PATH", path);
+        cmd.env("CARGO", &self.initial_cargo);
         cmd
     }
 
-- 
2.44.0

@Rajveer100
Copy link
Contributor Author

Thanks @onur-ozkan!

@Rajveer100 Rajveer100 force-pushed the branch-for-issue-123227 branch from 1e5adbd to 66dee4a Compare April 3, 2024 11:47
@onur-ozkan
Copy link
Member

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Apr 3, 2024

📌 Commit 66dee4a has been approved by onur-ozkan

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 3, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 3, 2024
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#123209 (Add section to sanitizer doc for `-Zexternal-clangrt`)
 - rust-lang#123342 (x.py test: remove no-op --skip flag)
 - rust-lang#123382 (Assert `FnDef` kind)
 - rust-lang#123386 (Set `CARGO` instead of `PATH` for Rust Clippy)
 - rust-lang#123393 (rustc_ast: Update `P<T>` docs to reflect mutable status.)
 - rust-lang#123394 (Postfix match fixes)
 - rust-lang#123412 (Output URLs of CI artifacts to GitHub summary)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 29c72ce into rust-lang:master Apr 3, 2024
@rustbot rustbot added this to the 1.79.0 milestone Apr 3, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 3, 2024
Rollup merge of rust-lang#123386 - Rajveer100:branch-for-issue-123227, r=onur-ozkan

Set `CARGO` instead of `PATH` for Rust Clippy

Resolves rust-lang#123227

Previously, clippy was using `cargo` from `PATH`, but since [PR](rust-lang/rust-clippy#11944), it now prioritises checking `CARGO` first.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

set CARGO instead of PATH
6 participants